Skip to content

plugins: require cloud_unsupported_reason and reject placeholder versions#4384

Open
twmb wants to merge 1 commit into
mainfrom
plugins-cloud-reason-and-versions
Open

plugins: require cloud_unsupported_reason and reject placeholder versions#4384
twmb wants to merge 1 commit into
mainfrom
plugins-cloud-reason-and-versions

Conversation

@twmb
Copy link
Copy Markdown
Contributor

@twmb twmb commented May 1, 2026

Adds a cloud_unsupported_reason column to internal/plugins/info.csv so that every connector excluded from the cloud distribution carries a one-line rationale alongside the cloud=n flag, and drops the version column entirely (157 of 320 rows carried the 0.0.0 placeholder, and nothing in the runtime, docs, or cloud bundling consumed the value).

A new TestPluginCloudEnablement in internal/plugins/alltest enforces the invariant going forward: every plugin must either be cloud-enabled or carry a non-empty cloud_unsupported_reason.

The reasons for connectors deliberately gated on security review are bucketed into "security: arbitrary code execution", "security: local filesystem access", "security: raw socket access", "security: opens a listener unreachable from cloud", and "security: pipeline stdio not exposed in cloud". The remaining reasons cover deprecated upstream protocols, managed metrics/tracing in cloud, missing certification, and a few connector-specific limitations.

@mmatczuk
Copy link
Copy Markdown
Contributor

mmatczuk commented May 4, 2026

I'd consider completely redoing this file or removing it completely. Having said that such a file comes in handy as a single source of truth. The problem is that data in component registration and cvs may and do diverge.

@JakeSCahill
Copy link
Copy Markdown
Contributor

having this data in rpk connect list would be useful too. We'd also like a deprecated_reason to be present when fields or connectors are deprecated.

@twmb
Copy link
Copy Markdown
Contributor Author

twmb commented May 6, 2026

@JakeSCahill we'll follow up on this — exposing cloud_unsupported_reason (and a deprecated_reason) via rpk connect list requires a corresponding change in benthos to add the fields to service.ConfigSpec so they flow through the schema and into the list output. Tracking it as a follow-up rather than blocking this PR.

@twmb twmb force-pushed the plugins-cloud-reason-and-versions branch from c0df421 to 014007f Compare May 6, 2026 16:58
Comment thread internal/plugins/info.csv Outdated
Comment thread internal/plugins/info.csv Outdated
Comment thread internal/plugins/info.csv Outdated
@claude

This comment was marked as resolved.

@twmb twmb force-pushed the plugins-cloud-reason-and-versions branch from 014007f to 4412edd Compare May 6, 2026 17:18
@claude

This comment was marked as resolved.

Adds a cloud_unsupported_reason column to internal/plugins/info.csv so that
every connector excluded from the cloud distribution carries a one-line
rationale alongside the cloud=n flag.

The version column was a placeholder field that, in practice, diverged from
component registration: 157 of 320 entries carried the 0.0.0 sentinel, and
nothing in the runtime, docs, or cloud bundling consumed the value. New
entries also had to be hand-tagged with the upcoming release at registration
time, which is the wrong moment to make that decision.

Drop the column entirely and remove the matching field on PluginInfo, the
basePluginInfo default, the columnInfo entry, and TestPluginVersion. The
cloud_unsupported_reason column and TestPluginCloudEnablement from the
preceding commit on this branch remain.

- TestPluginCloudEnablement requires every plugin to either be cloud-enabled
  or to carry a non-empty cloud_unsupported_reason; cloud-enabled rows must
  leave the reason empty.

The reasons for the connectors deliberately gated on security review are
bucketed into "security: arbitrary code execution", "security: local
filesystem access", "security: raw socket access", "security: opens a
listener unreachable from cloud", and "security: pipeline stdio not exposed
in cloud". The remaining reasons cover deprecated upstream protocols,
managed metrics/tracing in cloud, missing certification, and a few
connector-specific limitations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@twmb twmb force-pushed the plugins-cloud-reason-and-versions branch from 4412edd to 2fd9bab Compare May 6, 2026 17:59
@claude
Copy link
Copy Markdown

claude Bot commented May 6, 2026

Commits
LGTM

Review
This PR drops the version column from internal/plugins/info.csv and adds a new cloud_unsupported_reason column, with a corresponding TestPluginCloudEnablement test enforcing that every plugin is either cloud-enabled or carries a non-empty rationale. The Version field is also removed from PluginInfo.

I verified that the Version field on PluginInfo is unused outside internal/plugins/info.go, so the removal is safe. The CSV-to-struct mapping in pluginInfoFromMap, the column metadata in pluginInfoMapColumns, and the toMap round-trip are all consistent. The new test in alltest/plugins_test.go correctly enforces the cloud/reason invariant in both directions.

Minor note: the PR description still references a TestPluginVersion test and version-tagging behavior that the final commit explicitly removes — worth tightening up before merge for future readers.

LGTM

@twmb
Copy link
Copy Markdown
Contributor Author

twmb commented May 6, 2026

(pr desc updated)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants